-
Notifications
You must be signed in to change notification settings - Fork 26.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support de-deduping head tags by setting key #3170
Conversation
40cd626
to
58b724c
Compare
For reference #668 was my initial implementation. Will work on this sometime soon. Thanks for the PR with tests @liweinan0423 will combine the 2 👌 🙌 |
1a61a3d
to
b77ad4a
Compare
@liweinan0423 thanks for using my solution. The test you added fails in appveyor (windows). I'm pretty sure it's related to the test and not the code being broken in this case 👍 |
@timneutkens tests were good but the de-duping logic was wrong. I have fixed them. Also could you also delete my README texts and add your documentation, I think your docs are better. 👍 |
expect(html.includes('<link rel="stylesheet" href="/dup-style.css" class="next-head"/><link rel="stylesheet" href="/dup-style.css" class="next-head"/>')).toBeTruthy() | ||
expect(html.includes('<link rel="stylesheet" href="dedupe-style.css" class="next-head"/>')).toBeTruthy() | ||
expect(html.includes('<link rel="stylesheet" href="dedupe-style.css" class="next-head"/><link rel="stylesheet" href="dedupe-style.css" class="next-head"/>')).toBeFalsy() | ||
expect(html).toContain('<meta charSet="iso-8859-5" class="next-head"/>') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.toContain
Nice 💯
@liweinan0423 feel free to copy them over 🙌 |
@timneutkens any plan on merging this PR into |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Thank you very much
@liweinan0423 I'll do a new canary release this weekend. |
This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread. |
Fixes #3169